fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner - #16863
Conversation
…ead of sampling it The `single`-posture first-boot promotion read `sys_user` with no `orderBy` and a cap of 50, then sorted that array client-side. So "the oldest authenticable user" meant the oldest authenticable user among whatever 50 rows the driver produced first, and a client-side sort cannot notice: it sorts a sample and reports a global answer. Measured on 113 seeded users with the intended owner inserted first, holding the oldest created_at and an id that collates last: the in-memory driver returned it in row 1 and promoted it; the default sqlite driver returned id order, never saw it, and gave the unscoped admin_full_access grant — plus, through claimSeedOwnership, ownership of every seeded business record — to a seeded job-seeker persona. Same code, same config, same data. PLATFORM_OWNER_EMAIL_ENV was imported into this same file and read only on the walled branch, so a deployment that had declared its owner could still have somebody else promoted. Both halves land together: ordering alone still promotes someone the operator never chose, and honouring the declaration alone leaves the no-declaration path sorting a truncated unordered sample. - the candidate read carries orderBy created_at asc, id asc, and no client-side re-sort is left behind - a declared, authenticable, human holder of an OS_PLATFORM_OWNER_EMAIL address is preferred; verified matches rank ahead of unverified ones - a declared owner nobody can sign in as REFUSES loudly and promotes nobody, never falling back to whoever happens to be oldest - the bare cap 50 becomes a 200-row page with a 5000-row ceiling walked oldest-first, and reaching the ceiling warns with the number examined - the promotion log line and the returned report record the basis and the candidate-pool size Unchanged: no declaration still means promotion by age; a user nobody can authenticate as is still never promoted (#14348); an existing unscoped grant still short-circuits before any selection runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…rst-user-promotion-selection
…ared engine-double contracts The three doubles the new selection guard introduces are brought up to the contracts the repo's gates enforce, and the pinned ledger is grown so it actually protects this file: - update() routes through assertEngineUpdateDispatch, so a fixture drifting to a call shape ObjectQL.update would refuse fails loudly (check:engine-double-contract) - find() applies the caller's limit AFTER the filter and by presence (check:objectql-double-limit) - the WHERE matchers refuse a $-combinator by name instead of reading it as a field and answering false (check:where-matcher) scripts/engine-double-contract.pinned.json gains the new seams via `--write`; the shrink-only baseline is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…rst-user-promotion-selection
📓 Docs Drift CheckThis PR changes 1 package(s): 10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d54b3d70ae2723b45e7b075e8b646ae8755b6f41 && git checkout d54b3d70ae2723b45e7b075e8b646ae8755b6f41
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d127f9babce9d483ac4c053194360968e1c7ba0a ed6d4b2a85709cd8c7c0aa93c1e6d821e9a98053 && git checkout -B drift-repro d127f9babce9d483ac4c053194360968e1c7ba0a && git merge --no-ff ed6d4b2a85709cd8c7c0aa93c1e6d821e9a98053
node scripts/docs-audit/affected-docs.mjs --json d127f9babce9d483ac4c053194360968e1c7ba0a
|
Contract review (
|
… address under `single` The declared-owner leg ranked verified rows ahead of unverified ones and promoted an unverified holder where no verified one existed. Because `sys_user.email` is UNIQUE on the SQL family, a squatter who registered the declared address first could not be outranked by the real operator, who could not hold a row at all. Verification becomes a requirement: a declared address without a verified, authenticable, human holder refuses with zero grant rows and a warning naming the variable, the address and which half is missing. `shouldReplayBootstrapFor` admits the verifying `sys_user` update again, but only under `single` with an owner declared — the one configuration in which such a write can change this function's answer. Also regrades the changeset to `minor` (clause-② yes: `basis` is a new key on a published payload) and adds a candidate-page-crossing ordering case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rzQyhLGC5iVs11V3TzRs5
…rst-user-promotion-selection
维护者速读(终稿)— PR #16863 · #16682 · 只等你合并事情。 单组织( 改法(已按你 09-08 批 #100 的裁决 A 执行)。
验证。 新增 25 个用例跑在真实引擎 + 真实 sqlite 上,两条消融各自变红;CI 33 项全绿( 为什么要你亲手合。 总监席裁定:这是安全门(谁拿到无范围的 接受的代价。 声明了 owner 但没验证邮箱的 要你答的只有一个字:合不合?(是/否) Generated by Claude Code |
Fixes #16682
The
single-posture first-boot promotion readsys_userwith noorderByand a cap of 50, then sorted that array client-side. So "the oldest authenticable user" meant the oldest authenticable user among whatever 50 rows the driver produced first — a sample sorted and reported as a global answer. AndPLATFORM_OWNER_EMAIL_ENV, imported into that same file, was read only on the walled branch, so a deployment that had declared its owner could still have someone else promoted.Both halves land together, per triage's ruling: ordering alone still promotes someone the operator never chose, and honouring the declaration alone leaves the no-declaration path sorting a truncated unordered sample.
Clause-②: yes — bootstrapPlatformAdmin is re-exported by name from plugin-security/src/index.ts:33 and the diff adds basis?: 'declared-owner' | 'oldest-authenticable' to its returned object, so the published .d.ts gains a key on a published payload.
The maintainer ruling this round executes
Decision batch #100, 2026-09-08, recorded on #16682 (comment 5587754690). It supersedes the Choice 4A sentence "
singlenever readsOS_PLATFORM_OWNER_EMAIL" for this one point, and it settles the two findings the contract review held for a maintainer:singlepromotion. The rest of Choice 4A (platform-admin re-anchor L4 (plugin-security): bootstrap stops granting under walled postures; explain reports config-derived standing; deprecation log for legacy grants #11974) stands: retiring the walled write must not retire thesingleone, and the over-denial invariant (adminPromoted === truewith a grant row minted) stays pinned. The re-authored pin inbootstrap-platform-admin-walled-owner.test.tsnow quotes that ruling, where the previous revision quoted the triage seat's — which was exactly the reviewer's F3 objection.isHumanUser, it has asys_account, andemail_verified === true. Any miss refuses — zero grant rows, a warning naming the variable and the address — and there is no fall-back to oldest-authenticable while a declared address exists. The earlier "verified matches rank ahead of unverified ones" ordering is ruled moot; that bullet and its code are gone.singledeployment whose declared owner has not verified their email gets no platform admin at first boot until they do, loudly. It replaces a silent wrong promotion. The changeset says this too.Measured, on this branch, both drivers
113 seeded
sys_userrows, 7 holding credentials, the intended owner inserted first with the oldestcreated_atand an id that collates last,OS_PLATFORM_OWNER_EMAIL=admin@objectos.ai:InMemoryDriver)window[0] = usr_zzz_owner(insertion order)admin@objectos.aiadmin@objectos.aiSqlDriver, the default)window[0] = usr_ats_c001— the owner is not in the window at all (id order)candidate001@mail.exampleadmin@objectos.aiRe-measured with
OS_PLATFORM_OWNER_EMAILunset as well (the pure ordering leg): before, the same split; after,admin@objectos.aion both drivers,basis: oldest-authenticable. The raw unordered 50-row window is still driver-shaped after the change — that is a fact about the drivers, and the file pins it as an anti-vacuity case. What is no longer driver-shaped is the answer.What changed
created_atascending withidas tie-breaker (seeded populations routinely share one timestamp, and among ties an unordered read is exactly the sample-dependent answer this fixes). There is deliberately no client-side re-sort left behind: one would re-rank the returned page and keep the guard passing if the ordering were ever lost again.email_verifiedholder of anOS_PLATFORM_OWNER_EMAILaddress is the target.isHumanUserstill applies, so a declared address on arole: 'system'row is not a route to the grant.reason: 'declared_owner_not_authenticable'or'declared_owner_not_verified', a warning naming the variable, the addresses and which half is missing, and no grant row at all. No silent fall-back to whoever happens to be oldest; that is the outcome the card is about.shouldReplayBootstrapForadmits asys_userupdate touchingemail/email_verifiedundersingle— but only while an owner is declared. That is the one configuration in which such a write can change this function's answer, and it is what makes the ruling's "the replay predicate promotes as soon as verification lands" true rather than aspirational. platform-admin re-anchor L4 (plugin-security): bootstrap stops granting under walled postures; explain reports config-derived standing; deprecation log for legacy grants #11974 narrowed this arm away on the stated grounds that "singlepromotes the oldest authenticable human and never readsemail/email_verified" — the ruling makes that clause false for exactly this configuration and for no other, so with no owner declared the narrowing is untouched.The cap's disposition
Replaced, not merely raised, and never silent again. The bare
50becomesPLATFORM_ADMIN_CANDIDATE_PAGE_SIZE = 200walked oldest-first with a hardPLATFORM_ADMIN_CANDIDATE_SCAN_CEILING = 5000.sys_user/sys_accountinsert until an admin exists (shouldReplayBootstrapFor), so an unbounded scan would be a per-sign-up full-table read on exactly the deployments that have not been promoted yet.orderBy: an ORDERED page holds the OLDEST rows, which is exactly the set the age rule ranks, so truncation can only bite when every one of the oldest 5000 humans is non-authenticable. The unordered50could drop the answer on a 51-row install.The upgraded log line — exact text
Prefix unchanged (existing readers match on it); the basis and the pool are appended, and repeated in
metaasbasis/candidatePoolSize/userIdfor structured sinks. The returned report carriesbasistoo.Truncation warning:
Landing order relative to PR #16805 (F2)
scripts/engine-double-contract.pinned.jsonis a shared ledger, and open PR #16805 regenerates it too. It is not a single-writer block:SINGLE_CLAIM_PATHSdeclares.objectui-shaonly, andcheck:single-claim-pathsis green on this PR. What it is, is a regenerate-on-the-merged-tree obligation, and it is order-independent: whichever of the two lands second re-mergesorigin/mainand re-runsnode scripts/check-engine-double-contract.mjs --writeon the merged tree, then commits whatever that produces. This branch has just discharged its half — #16592 and #16730 had already moved the file from 3845 to 3875 lines since the old merge base,origin/mainis merged in here, and the regeneration on the merged tree reports0 added or grown, 0 lostwith the file at 3880 lines (main's 3875 plus this PR's 5 rows for the two new doubles). Nothing here needs #16805 to land first.验收备注
Triage's rubric, copied, each condition with its evidence. All in
packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts(25 runtime cases) unless noted.idand first bycreated_at, then assert the same user is promoted across driver row orders. ✅ The113-row fixture is the card's, verbatim.AS_RETURNED,INSERTION,REVERSED), all on the real engine over the real better-sqlite3 driver, plus a case asserting all three agree. An anti-vacuity case proves the unordered 50-row window really does hide the owner on the real driver, so the file cannot pass for a reason unrelated to the repair.OS_PLATFORM_OWNER_EMAILset to a user that is neither the oldest (itscreated_atis the newest of 113) nor in the first 50 rows (its id collates last), and which is the population's one verified row; asserted promoted under all three orders, withbasis: declared-owner, and explicitly not the oldest authenticable human. ✅OS_PLATFORM_OWNER_EMAIL时回落到最老的可认证用户,且两驱动一致 — a fixture deliberately different from condition 1, where the oldest authenticable row is not the id-last row, so "oldest wins" and "the owner happens to sort last" are separated. Same answer under all three orders. ✅min(created_at)fails here while condition 1 stays green. And a 60-row population where nobody can authenticate promotes nobody (no_authenticable_user, zero grant rows, and no warning, because that population is far under the ceiling). ✅OS_PLATFORM_OWNER_EMAIL但该邮箱没有可用账号 — two sub-cases, nosys_userrow at all and a verified row with nosys_account. Both refuse:declared_owner_not_authenticable, a warning naming the variable, the address and the phrase "NOT falling back to the oldest", and zero grant rows written. ⛔ No silent fall-back. ✅already_have_adminbefore any selection runs, so a declared owner cannot re-point an existing platform admin. ✅metafields, and that the published prefix still matches. ✅:426那处精确读 — untouched.git diffover the whole change contains no edit to that line (it now sits at:491, byte-identical). ✅The ruling's pins, in both directions
with NOBODY verified, the declared-owner leg REFUSES(declared_owner_not_verified,grants()empty, warning naming the variable, the address,VERIFIEDand "NOT falling back to the oldest"). This case is the re-authored predecessor ofCONTROL: with nobody verified, the oldest holder of the declared address wins, which asserted precisely the outcome the ruling refuses.among rows holding the declared address, only the VERIFIED one is eligible, plus the three-order declared-owner arm and the walled-owner suite's over-denial guard.the refusal LIFTS the moment verification lands: the first pass refuses with zero grant rows, anemail_verifiedupdate lands, the replayed pass promotes the same row withbasis: declared-owner. The trigger predicate's own half is pinned beside its producer inbootstrap-platform-admin-walled-owner.test.ts(fires on the verifying update once an owner is declared; still does not fire with none declared, nor under a walled posture, nor for aname-only update, nor onsys_account).declared_owner_not_authenticablecases are kept — both of them, unchanged in reason code. Authenticability is asked before verification precisely so plugin-security promotes the OLDEST human sys_user row, so an app that seeds a people directory grants platform admin to a row nobody can log in as #14348's refusal keeps its own name and its own pins, and the fixture for the no-sys_accountcase is now verified so it isolates that axis alone.F5 — the ordering guard now crosses a page boundary
The card's 113-row fixture fits inside one
PLATFORM_ADMIN_CANDIDATE_PAGE_SIZEpage, so it catches a lostorderByonly because there is no client-side re-sort left in the selection; a future defensive.sort()over the returned page would put every one of those cases back to green with the ordering gone. Added: one case atPLATFORM_ADMIN_CANDIDATE_PAGE_SIZE + 1rows with the owner's id collating last, so an unordered read puts the owner on page two — outside anything a page-local re-sort can reach — while page one already holds an authenticable row for the loop to stop on. It fails on a lostorderBywhether or not a re-sort is reintroduced, and it carries its own anti-vacuity assertion that the population really does exceed one page.F6 — the refusal warning re-emits, and that is accepted
With the variable set and the owner not yet promotable, every
sys_user/sys_accountinsert (and now the verifying update) replays the pass and re-emits the full refusalwarn— one warning per replay until the owner appears. Accepted, deliberately unlatched, and said so in the changeset: areportLegacyPlatformAdminGrant-style once-per-process latch would silence the one line that tells a fresh operator why their deployment has no administrator, on exactly the boots where they are still reading the log. The cost is repetition in a log nobody is paging; the cost of the latch is a silent, permanently admin-less install.F8 — untouched
Whether the literal two-driver test arm is wanted remains the maintainer's open question; it is not attempted here.
Declared deviation: the memory-driver arm is measured, not pinned
Condition 1 asks for the assertion on the memory driver and sqlite.
@objectstack/driver-memorycannot be imported into this suite, for two reasons that are each outside a repair's authority:packages/plugins/plugin-security/package.json— claimed by open PR fix(plugin-security)!: evaluate the insert-side RLScheckon the row that will be stored, afterbeforeInsert#16805 under the single-writer rule, which the dispatch names as a stop-and-report.scripts/driver-memory-census.ledger.json, and a test consumer's only fitting axis isruled-permanent, which the ledger states is "a maintainer ruling and lives inruledConsumers; nothing else may claim it." The gate's own header says a third arrival "is now refused at the gate".So the real memory-driver readings are in the table at the top of this PR — taken out-of-tree on this branch, before and after, and reported here rather than pinned. What the suite pins in their place is the property those two drivers were standing in for, over more orders than they produce between them, with the un-permuted arm being an ordinary real-driver run. If the maintainer wants the literal two-driver arm, it needs the ledger entry and the manifest edit, and both are theirs to make.
The facade is honest by construction: it permutes a result only when the query carried no
orderBy— which is precisely the freedom a driver has there — and forwards an ordered query verbatim, returning the real SQL engine's rows untouched. It never sorts. So a fix that sentorderByto a driver that ignored it would still be caught.The re-authored pin, now backed by a maintainer ruling
bootstrap-platform-admin-walled-owner.test.tscarried a case asserting the exact behaviour this change reverses:It is #11974's over-denial guard, and what it guards is unchanged: retiring the WALLED write must not retire the
singleone —adminPromoted === truewith a grant row actually minted, still asserted. What changed is the incumbent it happened to snapshot alongside that invariant, and the file header's "byte-for-byte" wording.The previous revision of this PR re-authored that case quoting the #16682 triage seat's ruling, and the contract review was right to refuse it (F3): a pin recorded under a maintainer ruling cannot be rewritten under a seat's. The quotation in the test is now the maintainer ruling of 2026-09-08 (batch #100) that resolved it, and the comment says in as many words which quotation it replaced and why.
Security boundary
This narrows who receives
admin_full_access, which is the ruled direction. Three things worth naming rather than leaving to be found:sys_user.emailis UNIQUE on the SQL family the real operator could not even coexist to outrank them. Under the ruling that squat is not a candidate at all — the leg refuses and nobody is promoted. Every declared-owner outcome is now a strict narrowing of the incumbent.packages/cli'sos meta resynccalls this function and therefore inherits the declared-owner rule, verification requirement included. Intended, and consistent.Verification
Final head
ed6d4b2a85, an ordinary merge commit oforigin/main(merge basedd2fd2003; ⛔ no history rewriting). Every number below was taken on that commit —git rev-parse --short HEAD=ed6d4b2a85at the time of the runs.pnpm --filter @objectstack/plugin-security exec vitest runover the three suites this change touches (bootstrap-platform-admin-promotion-selection.test.ts,bootstrap-platform-admin-walled-owner.test.ts,security-plugin.test.ts) — 3 files, 291 tests, all passing.pnpm --filter @objectstack/plugin-security typecheck— clean.pnpm --filter @objectstack/plugin-security test— 103 files, 1928 tests, all passing (1925 before this round; +3 is exactly the two cases added to the selection suite and the one added to the walled-owner suite).pnpm lint— the full repo-wideeslint . --no-inline-config, exit 0 over 6395 files, 0 errors and 0 warnings (--format json, counted from its own output). Not narrowed, so no narrowing evidence is owed.node scripts/pm/dispatch-gates.mjs --ran, reconciled against the same derivation, re-derived after a freshgit fetch origin mainand byte-identical). 64 exit 0. Zero findings (exit 1). The three non-zero are all exit 3, PREREQUISITE NOT MET —check:dual-build-cjs-loads,check:i18nandcheck:type-check-debteach require a full workspace build that CI does before them, and each says in its own words that this is "NOT a pass" and nothing was measured.node scripts/check-engine-double-contract.mjs --writeon the merged tree —775 (file, verb) row(s), 0 added or grown, 0 lostand6 seam row(s), 0 added or grown, 0 lost; the ledger is at 3880 lines andgit statusis clean, i.e. the merge already carried both sides.node scripts/pm/check-clause2-carriers.mjs --pair 16863— exit 0, "the clause-② declaration is readable in the fixed spelling and both carriers agree".needs:contract-reviewverified present on both carriers by read-back (PR fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863 and card plugin-security: the first-user promotion picks the oldest authenticable user from an UNORDERED 50-rowsys_userwindow, so on the default driver a seeded job seeker became platform admin and owned every seeded row #16682). The PR stays draft: maintainer-merged, never enqueued and never auto-merge-armed.docs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md): none touched, so no maintainer-speed-read section is owed here.Ablation — this round's two enforcement points, each proven able to fail
HEAD. The suite imports./bootstrap-platform-admin.js— a sibling module inside the same package — so vitest resolves the subject tosrc/, not to a builtdist/: no rebuild is involved in these legs and the dist preflight does not apply to them. Each leg proved its mutation reached disk before running anything (anchorgrep -cF1 to 0, injected marker 0 to 1, andgit hash-objectdiffering from the HEAD blob73543200badc1861fedeecea599dbb8dd1fdcd6d), and each restore is proven, not assumed (blob back to that exact hash and an emptygit diff HEAD), under atrap 'restore' EXIT INT TERMnaming a restore function, with absolute paths throughout.if (!isEmailVerifiedUserRow(row)) continue;on the declared-owner legthe refusal LIFTS the moment verification landsOLDEST_FIRSTfrom the leg-2 candidate readwith OS_PLATFORM_OWNER_EMAIL unset the fallback holds, identically under every orderThe capture was
tail-bounded, so only the lastFAILline of each run is quoted above; the failure COUNTS are vitest's own summary line. The previous round's A1–A4 were taken on head141876814, before this ruling changed leg 1: A1 (ordering) is superseded by A6 in a stronger form, A2's subject (the declared-owner leg) has been re-authored and is superseded by A5, and A3 (truncation warning) and A4 (log line) cover code this round did not touch.Out of scope, filed
already_have_adminshort-circuit readssys_user_permission_setwith an UNORDERED cap of 50, so an existing unscoped platform admin can be missed and a SECOND one minted #16861 — the same defect class on the other read in this function:already_have_adminreadssys_user_permission_setwith an unordered cap of 50 and applies the deciding!organization_idpredicate client-side, so on an install with 50-plus organization-scoped grants ofadmin_full_accessthe existing unscoped holder can be missed and a second unscoped admin minted. Not folded in: triage scoped this card to the promotion read, and repairing that one changes when the short-circuit fires, which is permission-boundary behaviour needing its own tests.sys_user.emailcarries a UNIQUE index, so on the SQL family two rows can never hold one address — measured asSQLITE_CONSTRAINT_UNIQUEwhile writing the earlier tie-break's fixture, which is why the two duplicate-address cases run on a double. It is also the mechanism that made F4's squat unanswerable by preference, so it is now recorded in the source comment rather than only here.Generated by Claude Code
Generated by Claude Code